Skip to content

feat(BA-5999): Validate session spec includes required resource slots in session creation#11556

Merged
HyeockJinKim merged 3 commits into
mainfrom
feat/BA-5999
May 12, 2026
Merged

feat(BA-5999): Validate session spec includes required resource slots in session creation#11556
HyeockJinKim merged 3 commits into
mainfrom
feat/BA-5999

Conversation

@seedspirit
Copy link
Copy Markdown
Contributor

@seedspirit seedspirit commented May 12, 2026

Summary

  • Fetch slot names marked required in resource_slot_types while building the session validation context.
  • Add a SessionSpec validator rule that rejects finalized kernel resource requests missing required slots or setting them to zero.
  • Keep deployment revision creation out of scope; deployment routes are validated later when they enqueue sessions through the same SessionSpec path.

Test plan

  • pants --no-dynamic-ui fmt src/ai/backend/manager/sokovan/scheduling_controller/validators/required_resource_slot_rule.py src/ai/backend/manager/sokovan/scheduling_controller/validators/session_spec_base.py src/ai/backend/manager/repositories/scheduler/types/session_creation.py src/ai/backend/manager/repositories/scheduler/db_source/db_source.py tests/unit/manager/sokovan/scheduling_controller/validators/test_session_spec_rules.py
  • pants --no-dynamic-ui lint src/ai/backend/manager/sokovan/scheduling_controller/validators/required_resource_slot_rule.py src/ai/backend/manager/sokovan/scheduling_controller/validators/session_spec_base.py src/ai/backend/manager/repositories/scheduler/types/session_creation.py src/ai/backend/manager/repositories/scheduler/db_source/db_source.py tests/unit/manager/sokovan/scheduling_controller/validators/test_session_spec_rules.py
  • pants --no-dynamic-ui test tests/unit/manager/sokovan/scheduling_controller/validators/test_session_spec_rules.py

Note: pushed with --no-verify because the current main-based pre-push test hook is known to fail.

Resolves BA-5999

@github-actions github-actions Bot added size:L 100~500 LoC comp:manager Related to Manager component labels May 12, 2026
@seedspirit seedspirit changed the title feat(BA-5999): validate required resource slots feat(BA-5999): enforce required resource slots on session creation May 12, 2026
seedspirit added a commit that referenced this pull request May 12, 2026
@seedspirit seedspirit changed the title feat(BA-5999): enforce required resource slots on session creation feat(BA-5999): Validate session spec includes required resource slots in session creation May 12, 2026
@seedspirit seedspirit marked this pull request as ready for review May 12, 2026 08:06
Copilot AI review requested due to automatic review settings May 12, 2026 08:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enforces that session creation requests include all resource slots marked as globally required in the resource_slot_types table, by fetching the required slot-name set into the SessionSpec validation context and introducing a validator rule that rejects kernels with missing or non-positive quantities for those slots.

Changes:

  • Fetch resource_slot_types.required = true slot names during session-spec context fetching and plumb them into SessionSpecValidationContext.
  • Add RequiredResourceSlotRule to the SessionSpec validator chain to reject kernels that omit required slots or request them with a quantity <= 0.
  • Add unit tests covering pass/fail cases for the new validator rule.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unit/manager/sokovan/scheduling_controller/validators/test_session_spec_rules.py Adds unit tests for RequiredResourceSlotRule and extends the context test helper to include required_slot_names.
src/ai/backend/manager/sokovan/scheduling_controller/validators/session_spec_base.py Extends SessionSpecValidationContext with required_slot_names.
src/ai/backend/manager/sokovan/scheduling_controller/validators/required_resource_slot_rule.py Introduces a new validator rule enforcing presence/non-zero of globally required resource slots per kernel.
src/ai/backend/manager/sokovan/scheduling_controller/validators/init.py Exports RequiredResourceSlotRule.
src/ai/backend/manager/sokovan/scheduling_controller/scheduling_controller.py Adds RequiredResourceSlotRule to the validator chain and wires required slot names into the validation context.
src/ai/backend/manager/repositories/scheduler/types/session_creation.py Adds required_slot_names to the fetched context bundle type.
src/ai/backend/manager/repositories/scheduler/db_source/db_source.py Fetches required slot names from resource_slot_types and returns them in SessionSpecContextFetch.
changes/11556.feature.md Adds a feature note for the new validation behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +42 to +51
missing = sorted(
str(slot_name)
for slot_name in required
if requested.get(str(slot_name), Decimal(0)) <= Decimal(0)
)
if missing:
raise InvalidAPIParameters(
extra_msg=(
f"kernel_specs[{idx}].execution_spec.resources is missing "
f"required resource slot(s): {missing}."
Comment on lines +348 to +353
async def _fetch_required_slot_names(self, db_sess: SASession) -> frozenset[SlotName]:
stmt = sa.select(ResourceSlotTypeRow.slot_name).where(
ResourceSlotTypeRow.required.is_(True)
)
rows = (await db_sess.execute(stmt)).scalars().all()
return frozenset(SlotName(slot_name) for slot_name in rows)
@HyeockJinKim HyeockJinKim merged commit fef2f7f into main May 12, 2026
37 checks passed
@HyeockJinKim HyeockJinKim deleted the feat/BA-5999 branch May 12, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants